-
Notifications
You must be signed in to change notification settings - Fork 87
feat(hwi): add hardware wallet support #207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
2c28a68 to
067f8eb
Compare
Pull Request Test Coverage Report for Build 19265985308Details
💛 - Coveralls |
d3cfe8b to
60628b8
Compare
- enable offline signer for creating tx whenever hwi feature is enabled - make handle_offline_wallet_subcommand fn async to handle hwi operations - add connecting to hwi fn in utils
- add `register` wallet hwi subcommand
fix clippy issues
- add signing psbt with hardware wallet
- add ledger and coldcard integration - update hwi as top level command - update CHANGELOG rebase and fix clippy issues
- async-hwi requires libudev-sys for linux systems - remove skip_blocks param as it is no longer needed for cbf
| } | ||
|
|
||
| #[cfg(feature = "hwi")] | ||
| pub async fn connect_to_hardware_wallet( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the function connect_to_hardware_wallet should be named get_connected_hardware_devices instead ?
That's according to the purpose of the subcommand HwiSubCommand::Devices (returning the list of connected hardware devices).
If it's assumed the user will always have only one connected device then I'd suggest renaming the variant to just HwiSubCommand::Device
|
|
||
| /// External descriptor | ||
| #[arg(env = "EXT_DESCRIPTOR", short = 'e', long = "ext_descriptor")] | ||
| pub ext_descriptor: Option<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed some inconsistency with the option ext-descriptor, some commands uses ext-descriptor but the added feature uses ext_descriptor. Same for --database_type.
|
I have tested with most of the devices, expect BitBox since you're not providing the emulator feature, this makes the |
Description
fixes #194
Notes to the reviewers
display_addressmethod in the provided trait in async-hwi library is meant to trigger address generation by the connected device and display it on the device screen. Generated addresses are the same for both the app and device, so app addresses are returned for thehwi addresssubcommand.This feature was tested with
SpecterSimulatorandColdcard, although not allhwifeatures were successfully tested on coldcard.For signing with hwi, it does sign but do not finalized the
PSBT, so it has to be finalized before broadcasting.Changelog notice
Checklists
HWI Wallet Subcommands implementation
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
CHANGELOG.mdBugfixes: